Draft
Conversation
* Improve `FileUtils.forceDelete()` tests on Windows On Windows, the `DeleteFile` Win32 API has a little quirk: it refuses to delete files with the legacy **DOS read-only attribute** set. (Because apparently 99% of Windows users don’t realize that “deleting a file” is actually an operation on the *directory*, not the file itself 😉). So the usual drill is: clear the read-only flag first, then delete. * Until JDK 25, `File.delete()` did this for you behind the scenes: it quietly stripped the flag before calling into `DeleteFile`. That meant your file might be left behind (with the flag missing) if the *real* ACLs didn’t allow deletion. * From JDK 25 onward, `File.delete()` doesn’t touch the flag anymore. If the bit is set, `DeleteFile` fails, end of story. * `FileUtils.forceDelete()` already knows how to juggle the flag itself, so its behavior didn’t change. This PR: * Updates two tests that were (unfairly) comparing `File.delete` with `FileUtils.forceDelete`. With JDK 25, their expectations diverged. * Adds a new test to confirm that `FileUtils.forceDelete` restores the read-only flag if the actual deletion fails. > [!WARNING] > I didn’t develop this on a Windows box, so I couldn’t test it locally. Leaving this as a **draft PR** until CI tells us whether Windows agrees with me. * fix: always clear read-only bit * fix: check for `IOException`, not `AccessDeniedException` * feat: add holder for file and parent attributes Introduce a helper that snapshots file and parent directory attributes before `setReadOnly` is applied. If a deletion attempt fails, the holder can restore the original attributes to keep the filesystem state consistent. * fix: Spotbugs failure * fix: prefer POSIX to DOS attributes when both are available On Linux/Unix, both POSIX and DOS attribute views may be supported, while on Windows only DOS attributes are available. Check for POSIX first to ensure the correct view is used across platforms. * fix: revert read-only related changes
Bumps [ossf/scorecard-action](https://github.com/ossf/scorecard-action) from 2.4.2 to 2.4.3. - [Release notes](https://github.com/ossf/scorecard-action/releases) - [Changelog](https://github.com/ossf/scorecard-action/blob/main/RELEASE.md) - [Commits](ossf/scorecard-action@05b42c6...4eaacf0) --- updated-dependencies: - dependency-name: ossf/scorecard-action dependency-version: 2.4.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.30.4 to 3.30.6. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@303c0ae...64d10c1) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 3.30.6 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 4.7.3 to 4.8.0. - [Release notes](https://github.com/actions/dependency-review-action/releases) - [Commits](actions/dependency-review-action@595b5ae...56339e5) --- updated-dependencies: - dependency-name: actions/dependency-review-action dependency-version: 4.8.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Add missing `read` argument validation in `IOUtils` In #790 I introduced `IOUtils#checkIndexFromLength` calls to validate arguments across the codebase. Ironically, the `IOUtils` class itself was left out. This PR addresses that omission by adding argument validation to `IOUtils#read` and `IOUtils#readFully`. Key points: * Ensures consistency with the rest of Commons IO by validating `offset` and `length`. * Fixes inconsistent exception behavior: * Previously, `length < 0` resulted in an `IllegalArgumentException`. * `offset < 0` did not trigger validation and failed later with an `IndexOutOfBoundsException`. * With this change, both invalid cases are handled consistently and upfront. * fix: failing tests
The implementation of `IOUtils.toByteArray(InputStream, int, int)` added in #776 throws different exceptions depending on the requested size: * For request sizes larger than the internal chunk size, it correctly throws an `EOFException`. * For smaller requests, it incorrectly throws a generic `IOException`. This PR makes the behavior consistent by always throwing an `EOFException` when the stream ends prematurely. Note: This also affects `RandomAccessFiles.read`. Its previous truncation behavior was undocumented and inconsistent with `RandomAccessFile.read` (which reads as much as possible). The new behavior is not explicitly documented here either, since it is unclear whether throwing on truncation is actually desirable.
Better unit test messages
Reduce vertical space
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 3.30.6 to 4.30.7. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@64d10c1...e296a93) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.30.7 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps `commons.bytebuddy.version` from 1.17.7 to 1.17.8. Updates `net.bytebuddy:byte-buddy` from 1.17.7 to 1.17.8 - [Release notes](https://github.com/raphw/byte-buddy/releases) - [Changelog](https://github.com/raphw/byte-buddy/blob/master/release-notes.md) - [Commits](raphw/byte-buddy@byte-buddy-1.17.7...byte-buddy-1.17.8) Updates `net.bytebuddy:byte-buddy-agent` from 1.17.7 to 1.17.8 - [Release notes](https://github.com/raphw/byte-buddy/releases) - [Changelog](https://github.com/raphw/byte-buddy/blob/master/release-notes.md) - [Commits](raphw/byte-buddy@byte-buddy-1.17.7...byte-buddy-1.17.8) --- updated-dependencies: - dependency-name: net.bytebuddy:byte-buddy dependency-version: 1.17.8 dependency-type: direct:development update-type: version-update:semver-patch - dependency-name: net.bytebuddy:byte-buddy-agent dependency-version: 1.17.8 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
category/java/errorprone.xml/UselessOperationOnImmutable as it is scheduled for removal from PMD. PMD 8.0.0 will remove support for this Rule.
This PR is split from #799. The `CloseShieldChannel` implementation only inspects interfaces **directly** implemented by the given channel’s class, ignoring those inherited from its superclasses. As a result, proxies for types such as `FileChannel` does not expose any of the interfaces declared on `FileChannel` itself.
Sort members
* Fixes issues in `CloseShieldChannel`
Two bugs in the `CloseShieldChannel` helper make it unreliable in practice:
1. **Type-erasure bug in `T wrap(T)`**
The method signature only works correctly when `T` is an **interface** extending `Channel`.
Since Java’s type system doesn’t allow constraining `T` to “interface types only,” this could lead to unexpected runtime `ClassCastException`s even though the code compiles successfully.
2. **Incomplete interface discovery**
The implementation only inspected interfaces **directly** implemented by the given channel’s class, ignoring those inherited from its superclasses.
As a result, proxies for types such as `FileChannel` did not expose any of the interfaces declared on `FileChannel` itself.
#### Fixes
This PR addresses both issues:
* **Reworks the API signature**
* Replaces `T wrap(T)` with its erasure: `Channel wrap(Channel)`.
* Introduces a new overload: `T wrap(T, Class<T>)`, which allows callers to explicitly specify the interface type they expect.
This version fails fast with a clear `IllegalArgumentException` if the provided type is not an interface, instead of allowing a `ClassCastException` later.
* **Improves interface collection logic**
* Updates the implementation to include interfaces declared on superclasses, ensuring all relevant `Channel` interfaces are correctly proxied.
* Fixes interface discovery in `CloseShieldChannel`
This PR is split from #799.
The `CloseShieldChannel` implementation only inspects interfaces **directly** implemented by the given channel’s class, ignoring those inherited from its superclasses.
As a result, proxies for types such as `FileChannel` does not expose any of the interfaces declared on `FileChannel` itself.
* fix: add overloads for commons channel types
* fix: add `ByteChannel` overload to resolve ambiguity
* fix: Limit interfaces to those verified.
* fix: rollback previous test
* fix: Restore generic method
Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 4.8.0 to 4.8.1. - [Release notes](https://github.com/actions/dependency-review-action/releases) - [Commits](actions/dependency-review-action@56339e5...40c09b7) --- updated-dependencies: - dependency-name: actions/dependency-review-action dependency-version: 4.8.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4.30.7 to 4.30.8. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](github/codeql-action@e296a93...f443b60) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.30.8 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* Fix concurrency issue in `IOUtils.skip` This patch addresses a concurrency problem in `IOUtils.skip`, as reported in [COMPRESS-666](https://issues.apache.org/jira/browse/COMPRESS-666) and [COMPRESS-697](https://issues.apache.org/jira/browse/COMPRESS-697). Previously, `IOUtils.skip` relied on `InputStream#read` to skip bytes, using a buffer shared across **all** threads. Although `IOUtils.skip` itself does not consume the data read, certain `InputStream` implementations (e.g. `ChecksumInputStream`) may process that data internally. In concurrent scenarios, this shared buffer could be overwritten by another thread between the `read` and the subsequent internal processing (such as checksum calculation), leading to incorrect behavior. This change reverts commit c12eaff and restores the use of a **per-thread buffer** in `IOUtils.skip`, ensuring thread safety and correct behavior in concurrent environments. * Adds a reentrancy guard to the thread-local pool * Apply suggestion from @Copilot (1) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> * Apply suggestions from @Copilot (2) Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Like roxyOutputStream.setReference(OutputStream)
Add ProxyReader.unrwap()
Cp850 input bytes.
malformed or unmappable input bytes
unmappable input bytes
|
|
||
| @Override | ||
| public long skip(long n) throws IOException { | ||
| charsRead += n; |
Check failure
Code scanning / CodeQL
Implicit narrowing conversion in compound assignment High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 1 day ago
In general, to avoid implicit narrowing in compound assignments, ensure that both sides of the assignment have compatible, non-narrowing types. Either widen the destination variable’s type or explicitly perform a checked conversion that preserves semantics and handles out-of-range values safely.
Here, the simplest fix that preserves existing behavior is to keep charsRead as an int (to avoid changing the class’s public/observable types) and change the compound assignment to an explicit, range-checked update. We can compute the amount to add as an int, clamped to the remaining allowable range before overflow, and then update charsRead. Since the logic of this class already limits reading based on maxCharsFromTargetReader (also an int), bounding the increment is consistent with its purpose. Specifically, in skip(long n), instead of charsRead += n, we compute an int delta as n > Integer.MAX_VALUE ? Integer.MAX_VALUE : (int) n, then clamp that delta so charsRead + delta does not exceed Integer.MAX_VALUE, and finally do charsRead += delta. This removes the implicit narrowing and makes the cast explicit and controlled. All changes are confined to the skip method in src/main/java/org/apache/commons/io/input/BoundedReader.java; no new imports or other methods are needed.
Suggested changeset
1
src/main/java/org/apache/commons/io/input/BoundedReader.java
| @@ -120,7 +120,21 @@ | ||
|
|
||
| @Override | ||
| public long skip(long n) throws IOException { | ||
| charsRead += n; | ||
| final int delta; | ||
| if (n > Integer.MAX_VALUE) { | ||
| delta = Integer.MAX_VALUE; | ||
| } else if (n < Integer.MIN_VALUE) { | ||
| delta = Integer.MIN_VALUE; | ||
| } else { | ||
| delta = (int) n; | ||
| } | ||
| if (delta > 0 && charsRead > Integer.MAX_VALUE - delta) { | ||
| charsRead = Integer.MAX_VALUE; | ||
| } else if (delta < 0 && charsRead < Integer.MIN_VALUE - delta) { | ||
| charsRead = Integer.MIN_VALUE; | ||
| } else { | ||
| charsRead += delta; | ||
| } | ||
| return super.skip(n); | ||
| } | ||
|
|
Copilot is powered by AI and may make mistakes. Always verify output.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.